Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ci): fix acceptance tests #2158

Merged
merged 8 commits into from
Jun 28, 2023

Conversation

thevilledev
Copy link
Contributor

Description

Currently acceptance tests run against Kind are failing as seen on the previous run here. I ran them on my fork as well with the following setups and both failed:

Two issues here:

  • On k8s 1.25 an empty localhost seccomp profile is a no-op but errors on k8s 1.27 with Error: localhostProfile must be set if seccompProfile type is Localhost. Fixing it for 1.27 only makes it better for 1.25.
  • Pod backoff limit is 6 by default (see docs) not 0, which is why tests fail on both 1.25 and 1.27. This is one of the reasons why AKS acceptance tests (see example run) are failing as well.

This PR addresses the issues as follows:

  • Add a sample audit seccomp profile to .github/config/seccomp-profiles as documented on k8s docs. Happy to hear input if this is the right place to store these.
  • Add a custom deployment configuration in .github/config for Kind, in which the seccomp profiles directory is mounted upon cluster creation.
  • Use custom config in Kind GHA.
  • Add methods for identifying whether a test is run against a Kind cluster. Similar methods exist for GKE, AKS, Minikube and the like already.
  • Spin-off separate test for seccomp tests which require node-local seccomp policies. Run it only on Kind. Maybe this can be extended to cover other clusters as well, but I don't have any test clusters on those platforms.
  • Change pod backoff limit default expectations to match documentation.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

Release Note

Release note for CHANGELOG:

NONE

References

None

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@thevilledev thevilledev requested a review from a team as a code owner June 23, 2023 09:26
@alexsomesan
Copy link
Member

@thevilledev Thanks so much for spotting and fixing these issues. The changes look good to me. Awesome work!

Going to wait for the CI run to finish and merge if green.

@thevilledev
Copy link
Contributor Author

Happy to help!

Do we consider this as a pr/no-changelog kind of change? Not user-facing and only impacts test/CI files

@arybolovlev
Copy link
Contributor

Hi @thevilledev,

Thank you for fixing this! Could you please do rebase? Once it is done, I will merge this PR into the main branch.

Great job!

@thevilledev
Copy link
Contributor Author

Perfect, thanks @arybolovlev. Rebase done!

Copy link
Contributor

@arybolovlev arybolovlev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@arybolovlev arybolovlev merged commit 1b34396 into hashicorp:main Jun 28, 2023
@thevilledev thevilledev deleted the chore/fix-tf-acc branch June 28, 2023 18:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants